Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes httpclient vulnerability by replacing it with a newer alternative #788

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pedro93
Copy link

@pedro93 pedro93 commented May 4, 2022

Security vulnerabilities have been found in apache-httpclient:commons-httpclient:3.1.
Unfortunately, 3.1 is the latest version of this package.

The suggestion to resolve the vulnerability is to https://hc.apache.org/httpcomponents-client-5.1.x/ 

@jjoyce0510
Copy link

jjoyce0510 commented May 4, 2022

@li-kramgopa Can we pls get a review on this PR? This is causing severe vuln on dependent project scans.

Thanks!
John

@karthikrg
Copy link
Contributor

@jjoyce0510 why are your ci tests failing? Also can you please update the diff to remove the comment?
You also need to update CHANGELOG and the version.

@karthikrg
Copy link
Contributor

@mchen07 @evanw555 fyi ^

@jjoyce0510
Copy link

@pedro93 Please update the PR to address the above :)

@pedro93
Copy link
Author

pedro93 commented May 4, 2022

Already did, regarding the failing CI I may need some help from someone more familiar with project.

@pedro93 pedro93 force-pushed the fix/httpclient-vulnerability branch from ee1d35f to 44dd8e4 Compare May 4, 2022 20:55
@mchen07
Copy link
Contributor

mchen07 commented May 4, 2022

can somebody provide link to the vulnerability issue this is trying to solve?

@nickibi
Copy link
Contributor

nickibi commented May 4, 2022

@pedro93 @jjoyce0510 Can you please share the details about the security vulnerability?
Within Linkedin, there are many other repos also use this dependency. We need to check with InfoSec team and ask for their recommendation. We are not sure the issue or impact at this moment. Moreover, if we are going to replace it, we need to get InfoSec confirmation on which version would be right one.
Before that I don't think we could review this PR.

@pedro93
Copy link
Author

pedro93 commented May 4, 2022

Here is the doc: https://docs.google.com/document/d/1ycmmQsY73LUAguDjdJncpr_GQ8E1lMJwfsuIpd5sPl0/edit?usp=sharing
Please let me know if you can't access it. Thanks!

@shirshanka
Copy link

Here's a pasted transcript from the doc.

apache-httpclient : commons-httpclient : 3.1

Screen Shot 2022-05-04 at 4 49 12 PM

sonatype-2007-0004
Issue
sonatype-2007-0004
Severity
Sonatype CVSS 3:
7.5
CVE CVSS 2.0:
0.0
Weakness
Sonatype CWE:
770
Source
Sonatype Data Research
Categories
Data
Explanation
The Apache HttpComponents project, a library of low level Java components focused on HTTP and associated protocols, is vulnerable to a Denial of Service (DoS). The HttpParser class' readRawLine method performs unbound reads on HTTP POST data. If a new line character \n is not encountered, memory consumption is not limited, leading to a Denial of Service.
Detection
The application is vulnerable by using this component.
Recommendation
We recommend upgrading to a version of this component that is not vulnerable to this specific issue.
NOTE: Component commons-httpclient:commons-httpclient is not expected to have any further releases, including a fixed version. The component was relocated to org.apache.httpcomponents:httpclient. Therefore, users of the affected versions of commons-httpclient:commons-httpclient should consider upgrading to a non-vulnerable version of org.apache.httpcomponents:httpclient.
Root Cause
commons-httpclient-3.1.jarorg/apache/commons/httpclient/HttpParser.class[2.0-alpha3,)
Advisories
Project:
http://hc.apache.org/index.html
Project:
https://issues.apache.org/jira/browse/HTTPCLIENT-644
Project:
https://issues.apache.org/jira/browse/HTTPCORE-3
Project:
https://issues.apache.org/jira/browse/HTTPCORE-4
CVSS Details
Sonatype CVSS 3:
7.5
CVSS Vector:
CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H

@pedro93
Copy link
Author

pedro93 commented Jan 10, 2023

Hello,

Pinging back on this PR. Have you had any chance to evaluate the changes?

Thank you.

@karthikrg
Copy link
Contributor

@mchen07 can you check this please?

@karthikrg
Copy link
Contributor

@pedro93 can you please rebase with master and upload a new diff?

@pedro93
Copy link
Author

pedro93 commented Jan 10, 2023

I merged master into this PR, is that not enough?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants